Make EventBus subscription failures observable (#74)#82
Merged
Conversation
Add awaitable subscribe_async/unsubscribe_async APIs that propagate WebSocket transport errors to callers, and track outcomes of the fire-and-forget subscribe/unsubscribe paths so callers can inspect them. - subscribe_async raises on transport failure and rolls back the newly registered handler so the caller's view stays consistent with the transport state. - unsubscribe_async propagates errors from WebSocketPort.unsubscribe. - subscribe() still does not raise, but the scheduled subscribe task is recorded and exposed via pending_subscription(); its outcome is cached in subscription_failure(). - start() preserves the historic 'never raise on subscribe failure' contract, but now records the failure for inspection so a flaky event type can be detected without changing startup semantics. The existing test that locked in silent post-start failure behaviour is replaced with assertions on the new contract, and new tests cover post-start failure, recovery, rollback, multi-handler unsubscribe, and cancellation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #74.
Issue validity
Valid.
EventBus.subscribe()schedules_ensure_subscription()withasyncio.ensure_future()afterstart, and_ensure_subscriptioncatches every exception silently. The existing testtest_subscribe_failure_logged_not_propagatedexplicitly locked in that behaviour. A listener could appear registered at the public API level while the underlying WebSocket subscribe never succeeded, so events would be silently missed.Fix
Make subscription success/failure part of the API contract without breaking the existing fire-and-forget call sites.
async subscribe_async(event_type, handler)/async unsubscribe_async(event_type, handler)propagate transport errors.subscribe_asyncrolls back the newly registered handler if the first subscribe for an event type fails, so the caller's view never diverges from the transport state.subscribe()path now stores the scheduled task; callers can:pending_subscription(event_type)-> awaitable in-flight subscribe task, orNone.subscription_failure(event_type)-> last recorded transport exception, cleared on a successful retry.start()preserves the historic 'never raise on subscribe failure' contract (a single flaky event type should not abort startup), but now records the failure for inspection.Tests
The existing test
test_subscribe_failure_logged_not_propagatedis replaced withtest_subscribe_failure_during_start_is_recorded, which asserts the new contract. New coverage:test_subscribe_post_start_failure_is_observabletest_subscribe_post_start_recovers_on_retrytest_subscribe_async_propagates_failure_and_rolls_backtest_subscribe_async_before_start_is_batchedtest_subscribe_async_additional_handler_skips_ws_calltest_unsubscribe_async_propagates_transport_errorstest_unsubscribe_async_unknown_handler_is_nooptest_unsubscribe_async_does_not_release_ws_when_handlers_remaintest_pending_subscription_none_when_idletest_cancelled_subscription_task_clears_pendingChecks run
ruff check src tests-> cleanruff format --check src tests-> cleanmypy src-> clean (strict)pytest tests/ --cov=haclient --cov-fail-under=95-> 304 passed, 97.15% coverageBackward compatibility
subscribe()/unsubscribe()/start()signatures and lifecycle are unchanged. Existing callers see no behavioural difference unless they opt in tosubscribe_async/subscription_failure/pending_subscription.